Surface Retry-After so rate-limited visitors know when to retry#49
Surface Retry-After so rate-limited visitors know when to retry#49HMAKT99 wants to merge 2 commits into
Conversation
The form gateway already returns a Retry-After header on 429 responses, but two things kept it from reaching the visitor. The Worker never added Retry-After to Access-Control-Expose-Headers, so a cross-origin browser fetch could not read it, and the client discarded it regardless, showing only a generic Try again later message. Expose the header from the Worker and, on a 429, append a concrete hint (Try again in about N seconds/minutes) to the form status message so the visitor knows how long to wait instead of retrying immediately. Covered by a Worker test asserting the header is exposed on rate-limited responses.
|
While reading the form gateway I noticed the It's intentionally defensive — a missing/odd header just yields no hint, so current behavior is preserved. Added a worker test asserting the header is exposed; |
mberman84
left a comment
mberman84
left a comment
There was a problem hiding this comment.
The Worker correctly exposes Retry-After through CORS, and the integrated Worker suite passes. Two client-side fixes remain:
Math.round(seconds / 60)can understate the server's minimum delay by up to 29 seconds (for example, 3,569 seconds is shown as 59 minutes). Please useMath.ceil(seconds / 60)so the guidance never sends the visitor back before the limit expires.- This changes
site/script.jswithout changing the current versioned script URL. Please bump the shared script asset version everywhere it is emitted/validated and regenerate affected pages so cached visitors receive the new behavior.
Address review on the rate-limit hint: - Use Math.ceil when converting Retry-After seconds to the displayed minutes/seconds so the hint never sends a visitor back before the server's limit expires (e.g. 3,569s now reads 60 minutes, not 59). - Bump the shared script.js asset version to 20260621-retry-after across the homepage, Learn/Agents pages, the loop-page generator, and scripts/check.mjs, then regenerate the loop pages so cached visitors receive the new client behavior.
|
Both addressed: switched the seconds→minutes (and seconds) conversion to |
|
@mberman84 ready for another look — |
2 participants
Summary
The form gateway already computes and returns a
Retry-Afterheader on429responses (both the verification rate limiter and the per-IP hourly/daily limiter), but it never reached the visitor:Retry-Afteris not a CORS-safelisted response header, so a cross-originfetch()from the site can't read it unless it's listed inAccess-Control-Expose-Headers— which it wasn't.postProtectedForm()surfaced onlyresponseBody.error, so a rate-limited visitor saw "Try again later" with no sense of how long.Fix
worker/src/index.js): addAccess-Control-Expose-Headers: Retry-Afterto the CORS headers so the browser can read it.site/script.js): on a429, readRetry-Afterand append a concrete hint — e.g. "Try again in about 1 minute." — to the existing status message.worker/test/index.test.js): assert the header is exposed on rate-limited responses.The hint formatting is defensive: a missing or non-numeric header simply yields no hint, preserving today's behavior.
Verification
npm --prefix worker run check→ 14/14 tests pass.node scripts/check.mjs→Loop Library checks passed.node --check site/script.js